Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Correct Data Types for Setpoint Description, Selector, and HVAC #37

Conversation

daviddsapir
Copy link
Contributor

@daviddsapir daviddsapir commented Oct 22, 2024

This commit addresses the data type issues in the Setpoint and HVAC model messages, specifically for SetpointDescriptionDataType, SetpointDescriptionListDataSelectorsType, and HvacSystemFunctionSetpointRelationDataType. It also corrects invalid types in function_data_factory.go for FunctionTypeHvacOperationModeDescriptionListData and FunctionTypeHvacSystemFunctionDescriptionListData.

Changes include:

  1. Updating SetpointId in hvacSystemFunctionSetpointRelationListData to be a list, as specified in the EEBus SPINE Technical Specification Resource Specification.
  2. Correcting data types for fields in SetpointDescriptionDataType and SetpointDescriptionListDataSelectorsType also according to the Resource Specification.

Before this commit, sending FunctionTypeHvacOperationModeDescriptionListData and FunctionTypeHvacSystemFunctionOperationModeRelationListData failed due to invalid data types provided to createFunctionData in the factory.

Additionally, the SetpointId field needed to be a list, not a single value. According to the specification, an operation mode can have multiple setpoints (up to four), such as for the "auto" operation mode.

Sending FunctionTypeSetpointDescriptionListData also failed due to incorrect field data types.

With these fixes, requests for FunctionTypeSetpointDescriptionListData, FunctionTypeHvacOperationModeDescriptionListData, and FunctionTypeHvacSystemFunctionOperationModeRelationListData now work correctly.

Screen shots from the EEBus SPINE Technical Specification Resource Specification for quick reference

4.3.12.6 hvacSystemFunctionSetpointRelationListData

image

4.3.23.6 setpointDescriptionListData (for SetpointDescriptionDataType)

image

5.3.21.4.3 Available selectors (for SetpointDescriptionListDataSelectorsType)

image

@daviddsapir daviddsapir force-pushed the fix/hvac-data-types-and-setpoints-selector-and-description branch from b66b575 to b9f3c5e Compare October 22, 2024 08:41
This commit addresses the data type issues in the `Setpoint` and `HVAC` model messages, specifically
for `SetpointDescriptionDataType`, `SetpointDescriptionListDataSelectorsType`, and
`HvacSystemFunctionSetpointRelationDataType`. It also corrects invalid types in `function_data_factory.go`
for `FunctionTypeHvacOperationModeDescriptionListData` and `FunctionTypeHvacSystemFunctionDescriptionListData`.

Changes include:
1. Updating `SetpointId` in `hvacSystemFunctionSetpointRelationListData` to be a list, as specified in the EEBus
   SPINE Technical Specification Resource Specification.
2. Correcting data types for fields in `SetpointDescriptionDataType` and `SetpointDescriptionListDataSelectorsType`
   also according to the Resource Specification.

Before this commit, sending `FunctionTypeHvacOperationModeDescriptionListData` and
`FunctionTypeHvacSystemFunctionOperationModeRelationListData` failed due to invalid
data types provided to `createFunctionData` in the factory.

Additionally, the `SetpointId` field needed to be a list, not a single value.  According to the specification, an
operation mode can have multiple setpoints (up to four), such as for the "auto" operation mode.

Sending `FunctionTypeSetpointDescriptionListData` also failed due to incorrect field data types.

With these fixes, requests for `FunctionTypeSetpointDescriptionListData`, `FunctionTypeHvacOperationModeDescriptionListData`,
and `FunctionTypeHvacSystemFunctionOperationModeRelationListData` now work correctly.
@daviddsapir daviddsapir force-pushed the fix/hvac-data-types-and-setpoints-selector-and-description branch from b9f3c5e to d5f89c7 Compare October 22, 2024 08:47
@DerAndereAndi
Copy link
Member

Thanks for these findings and fixes. Very much appreciated!

@coveralls
Copy link

coveralls commented Oct 23, 2024

Coverage Status

coverage: 93.712%. remained the same
when pulling f75c7fe on daviddsapir:fix/hvac-data-types-and-setpoints-selector-and-description
into f5aacd9 on enbility:dev.

model/setpoint.go Outdated Show resolved Hide resolved
@DerAndereAndi
Copy link
Member

I think the failing test code also needs to include the foreign identifiers in the sample data structures for the merge to work correctly. Could you take a look at this?

@daviddsapir daviddsapir force-pushed the fix/hvac-data-types-and-setpoints-selector-and-description branch from 104d4e9 to f75c7fe Compare October 23, 2024 16:19
@daviddsapir
Copy link
Contributor Author

I’ve fixed the failing test

@DerAndereAndi
Copy link
Member

Awesome, thank you very much!

@DerAndereAndi DerAndereAndi merged commit 0b14938 into enbility:dev Oct 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants